unlock: Use deployment backing dir

Closes: https://github.com/ostreedev/ostree/issues/3391
Basically it's not uncommon to make `/var/tmp` a separate
partition, but this constrains the amount of data that
can be written to `/usr` when unlocking.

Change things here to write to the deployment's backing
dir which is part of the same rootfs as the storage
and is lifecycle bound to the deployment, ensuring
it gets GC'd.

Signed-off-by: Colin Walters <walters@verbum.org>

diff --git a/src/boot/ostree-tmpfiles.conf b/src/boot/ostree-tmpfiles.conf
index 69c2d3f385e3fa23f3bb68a794b071ce84a73316..711cd862deb06fb5627236593fe02a4f093895ed 100644 (file)
--- a/src/boot/ostree-tmpfiles.conf
+++ b/src/boot/ostree-tmpfiles.conf
@@ -16,4 +16,6 @@
 # ostree runtime configuration
 d /run/ostree 0755 root root -
 # https://github.com/ostreedev/ostree/issues/393
+# Note this can eventually be removed now that we
+# write to the deployment backing dir.
 R! /var/tmp/ostree-unlock-ovl.*

diff --git a/src/libostree/ostree-sysroot.c b/src/libostree/ostree-sysroot.c
index 3968c38fedea2f182577bc7fcfbe8108d99ffbfe..dab70486de9f7987ceffd82c3070801f655c45e4 100644 (file)
--- a/src/libostree/ostree-sysroot.c
+++ b/src/libostree/ostree-sysroot.c
@@ -2108,6 +2108,8 @@ ostree_sysroot_deployment_unlock (OstreeSysroot *self, OstreeDeployment *deploym
   if (!glnx_opendirat (self->sysroot_fd, deployment_path, TRUE, &deployment_dfd, error))
     return FALSE;
 
+  g_autofree char *backing_relpath = _ostree_sysroot_get_deployment_backing_relpath (deployment);
+
   g_autoptr (OstreeSePolicy) sepolicy = ostree_sepolicy_new_at (deployment_dfd, cancellable, error);
   if (!sepolicy)
     return FALSE;
@@ -2121,10 +2123,9 @@ ostree_sysroot_deployment_unlock (OstreeSysroot *self, OstreeDeployment *deploym
     usr_mode = stbuf.st_mode;
   }
 
-  const char *ovl_options = NULL;
+  g_autofree char *ovl_options = NULL;
   static const char hotfix_ovl_options[]
       = "lowerdir=usr,upperdir=.usr-ovl-upper,workdir=.usr-ovl-work";
-  g_autofree char *unlock_ovldir = NULL;
 
   switch (unlocked_state)
     {
@@ -2141,18 +2142,22 @@ ostree_sysroot_deployment_unlock (OstreeSysroot *self, OstreeDeployment *deploym
           return FALSE;
         if (!mkdir_unmasked (deployment_dfd, ".usr-ovl-work", usr_mode, cancellable, error))
           return FALSE;
-        ovl_options = hotfix_ovl_options;
+        ovl_options = g_strdup (hotfix_ovl_options);
       }
       break;
     case OSTREE_DEPLOYMENT_UNLOCKED_DEVELOPMENT:
     case OSTREE_DEPLOYMENT_UNLOCKED_TRANSIENT:
       {
-        unlock_ovldir = g_strdup ("/var/tmp/ostree-unlock-ovl.XXXXXX");
-        /* We're just doing transient development/hacking?  Okay,
-         * stick the overlayfs bits in /var/tmp.
-         */
-        const char *development_ovl_upper;
-        const char *development_ovl_work;
+        // Holds the overlay backing data in the deployment backing dir, which
+        // ensures that (unlike our previous usage of /var/tmp) that it's on the same
+        // physical filesystem. It's valid to make /var/tmp a separate FS, but for
+        // this data it needs to scale to the root.
+        g_autofree char *usrovldir_relative
+            = g_build_filename (backing_relpath, OSTREE_DEPLOYMENT_USR_TRANSIENT_DIR, NULL);
+        // We explicitly don't want this data to persist, so if it happened
+        // to leak from a previous boot, ensure the dir is cleaned now.
+        if (!glnx_shutil_rm_rf_at (self->sysroot_fd, usrovldir_relative, cancellable, error))
+          return FALSE;
 
         /* Ensure that the directory is created with the same label as `/usr` */
         {
@@ -2163,18 +2168,26 @@ ostree_sysroot_deployment_unlock (OstreeSysroot *self, OstreeDeployment *deploym
           if (!_ostree_sepolicy_preparefscreatecon (&con, sepolicy, "/usr", usr_mode, error))
             return FALSE;
 
-          if (g_mkdtemp_full (unlock_ovldir, 0755) == NULL)
-            return glnx_throw_errno_prefix (error, "mkdtemp");
+          // Create a new backing dir.
+          if (!mkdir_unmasked (self->sysroot_fd, usrovldir_relative, usr_mode, cancellable, error))
+            return FALSE;
         }
 
-        development_ovl_upper = glnx_strjoina (unlock_ovldir, "/upper");
-        if (!mkdir_unmasked (AT_FDCWD, development_ovl_upper, usr_mode, cancellable, error))
+        // Open a fd for our new dir
+        int ovldir_fd = -1;
+        if (!glnx_opendirat (self->sysroot_fd, usrovldir_relative, FALSE, &ovldir_fd, error))
           return FALSE;
-        development_ovl_work = glnx_strjoina (unlock_ovldir, "/work");
-        if (!mkdir_unmasked (AT_FDCWD, development_ovl_work, usr_mode, cancellable, error))
+
+        // Create the work and upper dirs there
+        if (!mkdir_unmasked (ovldir_fd, "upper", usr_mode, cancellable, error))
+          return FALSE;
+        if (!mkdir_unmasked (ovldir_fd, "work", usr_mode, cancellable, error))
           return FALSE;
-        ovl_options = glnx_strjoina ("lowerdir=usr,upperdir=", development_ovl_upper,
-                                     ",workdir=", development_ovl_work);
+
+        // TODO investigate depending on the new mount API with overlayfs
+        ovl_options = g_strdup_printf ("lowerdir=usr,upperdir=/proc/self/fd/%d/upper"
+                                       ",workdir=/proc/self/fd/%d/work",
+                                       ovldir_fd, ovldir_fd);
       }
     }
 
@@ -2249,7 +2262,7 @@ ostree_sysroot_deployment_unlock (OstreeSysroot *self, OstreeDeployment *deploym
         if (!glnx_shutil_mkdir_p_at (AT_FDCWD, devpath_parent, 0755, cancellable, error))
           return FALSE;
 
-        if (!g_file_set_contents (devpath, unlock_ovldir, -1, error))
+        if (!g_file_set_contents (devpath, "", -1, error))
           return FALSE;
       }
     }

diff --git a/src/libotcore/otcore.h b/src/libotcore/otcore.h
index ceeb1a9265501d27c2fbcef254b52575b980626f..5003d47ddc7d720ce00a3e58f17501c130d195df 100644 (file)
--- a/src/libotcore/otcore.h
+++ b/src/libotcore/otcore.h
@@ -89,6 +89,8 @@ ComposefsConfig *otcore_load_composefs_config (const char *cmdline, GKeyFile *co
 #define OSTREE_DEPLOYMENT_BACKING_DIR "backing"
 // The directory holding the root overlayfs
 #define OSTREE_DEPLOYMENT_ROOT_TRANSIENT_DIR "root-transient"
+// The directory holding overlayfs for /usr (ostree admin unlock)
+#define OSTREE_DEPLOYMENT_USR_TRANSIENT_DIR "usr-transient"
 
 // Written by ostree admin unlock --hotfix, read by ostree-prepare-root
 #define OTCORE_HOTFIX_USR_OVL_WORK ".usr-ovl-work"

diff --git a/tests/kolainst/destructive/unlock-transient.sh b/tests/kolainst/destructive/unlock-transient.sh
index 19b7b693ad973aa464e508c170289c5f9b7c94d0..6e8a28eb814c664d1103c2d49db49add82f1acfb 100755 (executable)
--- a/tests/kolainst/destructive/unlock-transient.sh
+++ b/tests/kolainst/destructive/unlock-transient.sh
@@ -6,6 +6,11 @@ set -xeuo pipefail
 
 testfile=/usr/share/writable-usr-test
 
+stateroot=$(rpmostree_query_json '.deployments[0].osname')
+checksum=$(rpmostree_query_json '.deployments[0].checksum')
+serial=$(rpmostree_query_json '.deployments[0].serial')
+backing=/ostree/deploy/${stateroot}/backing/$checksum.$serial/usr-transient
+
 case "${AUTOPKGTEST_REBOOT_MARK:-}" in
   "") 
     require_writable_sysroot
@@ -22,9 +27,17 @@ case "${AUTOPKGTEST_REBOOT_MARK:-}" in
     if touch ${testfile} || rm -v "${testfile}" 2>/dev/null; then
       fatal "modified ${testfile}"
     fi
+    # And the file should be written to the backing dir, note the
+    # /usr got stripped.
+    test -f ${backing}/upper/share/writable-usr-test
     /tmp/autopkgtest-reboot 2
     ;;
   "2")
+    if test -f "${testfile}"; then
+      fatal "${testfile} persisted across reboot?"
+    fi
+    ostree admin unlock --transient
+    # Test again to ensure we didn't leak state across reboot
     if test -f "${testfile}"; then
       fatal "${testfile} persisted across reboot?"
     fi